Skip to content

Add all accepted ILIAS 12 NPM dependency PRs into package.json#11262

Merged
mbecker-databay merged 1 commit into
ILIAS-eLearning:trunkfrom
lscharmer:12/npm-dependency-merge
Jun 2, 2026
Merged

Add all accepted ILIAS 12 NPM dependency PRs into package.json#11262
mbecker-databay merged 1 commit into
ILIAS-eLearning:trunkfrom
lscharmer:12/npm-dependency-merge

Conversation

@lscharmer
Copy link
Copy Markdown
Contributor

@lscharmer lscharmer commented Mar 13, 2026

Hi,
this PR merges all accepted ILIAS 12 NPM dependencies.

The following PRs are included:

For root package.json:

For components/ILIAS/Chatroom/chat/package.json:

Things to note

eslint + eslint-config-airbnb-base

This is the same issue as mentioned in: #9086
The package eslint-config-airbnb-base requires an out of date version of eslint (https://eslint.org/version-support/).

I opt for dropping it for now.

eslint is now downgraded to v8, which resolves this issue.

rollup on Mac

As mentioned & fixed by @thibsy in #9361, some packages (e.g. for mac) where not included with the normal npm install to create the package-lock.json.

Even though I created the package-lock.json the same way as last year, this time the packages are already included AFAIK:

# This PRs and output after PR #9361, are the same
$ grep rollup-darwin package-lock.json
    "node_modules/@rollup/rollup-darwin-arm64": {
      "resolved": "https://registry.npmjs.org/@rollup/rollup-darwin-arm64/-/rollup-darwin-arm64-4.53.3.tgz",
    "node_modules/@rollup/rollup-darwin-x64": {
      "resolved": "https://registry.npmjs.org/@rollup/rollup-darwin-x64/-/rollup-darwin-x64-4.53.3.tgz",
	"@rollup/rollup-darwin-arm64": "4.53.3",
	"@rollup/rollup-darwin-x64": "4.53.3",

vs. package-lock.json before PR #9361:

$ grep rollup-darwin package-lock.json
        "@rollup/rollup-darwin-arm64": "4.38.0",
        "@rollup/rollup-darwin-x64": "4.38.0",

@thibsy it would be great if you could verify this, I will test this too as soon as I have a Mac available.

Fixing Unit Tests

The change made in: components/ILIAS/UI/tests/Client/Table/Presentation/presentation.table.test.js fixes an endless loop error. With this change all JS Unit Tests run successfully.

Summary

In comparison to ILIAS 11:

New dependencies: None

Removed dependency:

@lscharmer lscharmer requested a review from thibsy March 13, 2026 11:26
@lscharmer lscharmer added technical board dependencies Pull requests that update a dependency file javascript Pull requests that update Javascript code labels Mar 13, 2026
Copy link
Copy Markdown
Contributor

@thibsy thibsy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @lscharmer,

Thx a lot for providing the integration of the dependency PR's and for your detailed summary. Much appreciated.

  • chart.js: @tfamula requested this last time (#8646), maybe @schmitz-ilias or @alex40724 can take over on behalf of Leifos? This would be much appreciated, I am not very familiar with the library in question, but we ultimately need it for the charts inside UI framework.
  • jstree: @alex40724 requested this last time (#10215), maybe he could do so again. But sine this is legacy UI, it could also be shepherded by someone else.
  • eslint-config-airbnb-base: this is my mistake, I should have requested eslint with v8. Since having no code-style is so much worse than using a deprecated development dependency, I would ask you to downgrade eslint and incorporate the eslint-config-airbnb-base package again. I also requested the same procedure last time, which got approved by JF (#9840).
  • rollup: works on my machine (macOS 26.3.1) – I think we're good this time =). Thx for inspecting.

Other code changes to the UI framework LGTM. Funny that this test case worked before?

Kind regards,
@thibsy

@thibsy
Copy link
Copy Markdown
Contributor

thibsy commented Mar 23, 2026

@ILIAS-eLearning/technical-board let me know if I should go to JF with this again.

@marvimarv
Copy link
Copy Markdown

@ILIAS-eLearning/technical-board let me know if I should go to JF with this again.

@thibsy: There is no need to go through the JF again, the dependency is accepted.

@lscharmer lscharmer force-pushed the 12/npm-dependency-merge branch 2 times, most recently from c4d1680 to f9deaa1 Compare March 24, 2026 10:46
@lscharmer
Copy link
Copy Markdown
Contributor Author

This PR is still missing 2 dependencies to be complete.
As mentioned by @thibsy above (#11262 (review)):

Best regards
@lscharmer

@lscharmer lscharmer force-pushed the 12/npm-dependency-merge branch from f9deaa1 to bdefa0b Compare June 1, 2026 12:59
@lscharmer
Copy link
Copy Markdown
Contributor Author

lscharmer commented Jun 1, 2026

Hi @ILIAS-eLearning/technical-board,
the dependencies jstree and chart.js are now integrated into this PR as well and working correctly.

Additionally all dependency versions are now prefixed with ^ to allow minor updates.

I also run npm audit (for chat as well) to fix all security issues (the major version of @rollup/plugin-terser changed but didn't break anything in my tests and is only used in dev)

This PR can IMO be merged now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file javascript Pull requests that update Javascript code technical board

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants